Skip to content

Update the arch_spec example with memory placement#282

Merged
ShangkunLi merged 7 commits intomainfrom
testbench
Mar 12, 2026
Merged

Update the arch_spec example with memory placement#282
ShangkunLi merged 7 commits intomainfrom
testbench

Conversation

@n0thingNoob
Copy link
Collaborator

#148
To determine the memory placement

@n0thingNoob
Copy link
Collaborator Author

@tancheng Is this ok? Or we still need to add some other things?

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates the arch_spec_example.yaml test/example architecture spec to illustrate off-chip memory placement intent (memory_side) alongside an explicit concrete realization via tile_overrides.

Changes:

  • Add memory_side: "West" as an architectural-intent annotation in cgra_defaults.
  • Remove memory ops from tile_defaults.fu_types and instead explicitly mark memory-capable tiles via tile_overrides.
  • Expand the example tile_overrides to show multiple west-edge tiles (CGRA (0,0)) having mem/mem_indexed.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Copy link
Contributor

@tancheng tancheng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why there is no test failing? It should fail a lot due to the mapping changes..

@tancheng
Copy link
Contributor

Why there is no test failing? It should fail a lot due to the mapping changes..

So indeed many tests failed?

@n0thingNoob
Copy link
Collaborator Author

Why there is no test failing? It should fail a lot due to the mapping changes..

So indeed many tests failed?

yes. I am working on it.

@ShangkunLi ShangkunLi merged commit 37bf430 into main Mar 12, 2026
1 check passed
@tancheng
Copy link
Contributor

I guess though many kernels are affected, the mappedII didn't change a lot? @ShangkunLi @n0thingNoob

@ShangkunLi
Copy link
Collaborator

I guess though many kernels are affected, the mappedII didn't change a lot? @ShangkunLi @n0thingNoob

Not changed too much, but the mapping time increased a lot orz....

@tancheng
Copy link
Contributor

I guess though many kernels are affected, the mappedII didn't change a lot? @ShangkunLi @n0thingNoob

Not changed too much, but the mapping time increased a lot orz....

For the one with longest compilation time, can you try also make the tiles on the first row to be able to access memory (not only the first column), to see whether the compilation time can be improved?

@ShangkunLi
Copy link
Collaborator

ShangkunLi commented Mar 12, 2026

I guess though many kernels are affected, the mappedII didn't change a lot? @ShangkunLi @n0thingNoob

Not changed too much, but the mapping time increased a lot orz....

For the one with longest compilation time, can you try also make the tiles on the first row to be able to access memory (not only the first column), to see whether the compilation time can be improved?

The configuration of this pr has already set that both the first col & row can access the memory. I think the mapping time introduced by this pr is acceptable (<5min in total). The mapping time introduced in #278 is much larger (~10 minutes for all tests).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants